Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reland "[AArch64] Decouple feature dependency expansion. (#94279)" #95519

Merged
merged 6 commits into from
Jun 18, 2024

Conversation

labrinea
Copy link
Collaborator

@labrinea labrinea commented Jun 14, 2024

This is the second attempt. When parsing the target attribute
we should be letting cc1 features which don't correspond to
Extensions pass through to avoid errors like the following:

% cat neon.c
__attribute__((target("arch=armv8-a")))
uint64x2_t foo(uint64x2_t a, uint64x2_t b) { return veorq_u64(a, b); }

% clang --target=aarch64-linux-gnu -c neon.c
error: always_inline function 'veorq_u64' requires target feature
       'outline-atomics', but would be inlined into function 'foo'
       that is compiled without support for 'outline-atomics'

Co-authored-by: Tomas Matheson Tomas.Matheson@arm.com

labrinea added 2 commits June 14, 2024 08:54
This is the second attempt. We should be inserting the Driver
features in front of the features of a parsed target attribute
to avoid errors like the following:

```
% cat neon.c
__attribute__((target("arch=armv8-a")))
uint64x2_t foo(uint64x2_t a, uint64x2_t b) { return veorq_u64(a, b); }

% clang --target=aarch64-linux-gnu -c neon.c
error: always_inline function 'veorq_u64' requires target feature
       'outline-atomics', but would be inlined into function 'foo'
       that is compiled without support for 'outline-atomics'
```
* We should be inserting the Driver features in front of the features
  of a parsed target attribute.
* Added a sema test.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 14, 2024

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-clang

Author: Alexandros Lamprineas (labrinea)

Changes

This is the second attempt. We should be inserting the Driver
features in front of the features of a parsed target attribute
to avoid errors like the following:

% cat neon.c
__attribute__((target("arch=armv8-a")))
uint64x2_t foo(uint64x2_t a, uint64x2_t b) { return veorq_u64(a, b); }

% clang --target=aarch64-linux-gnu -c neon.c
error: always_inline function 'veorq_u64' requires target feature
       'outline-atomics', but would be inlined into function 'foo'
       that is compiled without support for 'outline-atomics'

Patch is 43.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95519.diff

12 Files Affected:

  • (modified) clang/include/clang/AST/ASTContext.h (-3)
  • (modified) clang/lib/AST/ASTContext.cpp (+26-23)
  • (modified) clang/lib/AST/CMakeLists.txt (+2)
  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+33-72)
  • (modified) clang/lib/Basic/Targets/AArch64.h (-4)
  • (modified) clang/test/CodeGen/aarch64-cpu-supports-target.c (+2-2)
  • (modified) clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp (+1-1)
  • (modified) clang/test/CodeGen/aarch64-targetattr.c (+20-28)
  • (modified) clang/test/CodeGen/attr-target-version.c (+23-23)
  • (modified) clang/test/Sema/aarch64-neon-target.c (+8-2)
  • (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+65-42)
  • (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+33-18)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 53ece996769a8..f1f20fca477a4 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3210,9 +3210,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// valid feature names.
   ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD) const;
 
-  std::vector<std::string>
-  filterFunctionTargetVersionAttrs(const TargetVersionAttr *TV) const;
-
   void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
                              const FunctionDecl *) const;
   void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 34aa399fda2f8..7fb5966c0bf7c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -87,6 +87,7 @@
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/TargetParser/AArch64TargetParser.h"
 #include "llvm/TargetParser/Triple.h"
 #include <algorithm>
 #include <cassert>
@@ -13676,17 +13677,20 @@ QualType ASTContext::getCorrespondingSignedFixedPointType(QualType Ty) const {
   }
 }
 
-std::vector<std::string> ASTContext::filterFunctionTargetVersionAttrs(
-    const TargetVersionAttr *TV) const {
-  assert(TV != nullptr);
-  llvm::SmallVector<StringRef, 8> Feats;
-  std::vector<std::string> ResFeats;
-  TV->getFeatures(Feats);
-  for (auto &Feature : Feats)
-    if (Target->validateCpuSupports(Feature.str()))
-      // Use '?' to mark features that came from TargetVersion.
-      ResFeats.push_back("?" + Feature.str());
-  return ResFeats;
+// Given a list of FMV features, return a concatenated list of the
+// corresponding backend features (which may contain duplicates).
+static std::vector<std::string> getFMVBackendFeaturesFor(
+    const llvm::SmallVectorImpl<StringRef> &FMVFeatStrings) {
+  std::vector<std::string> BackendFeats;
+  for (StringRef F : FMVFeatStrings) {
+    if (auto FMVExt = llvm::AArch64::parseArchExtension(F)) {
+      SmallVector<StringRef, 8> Feats;
+      FMVExt->DependentFeatures.split(Feats, ',', -1, false);
+      for (StringRef F : Feats)
+        BackendFeats.push_back(F.str());
+    }
+  }
+  return BackendFeats;
 }
 
 ParsedTargetAttr
@@ -13745,32 +13749,31 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
                     Target->getTargetOpts().FeaturesAsWritten.end());
     Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
   } else if (const auto *TC = FD->getAttr<TargetClonesAttr>()) {
-    std::vector<std::string> Features;
     if (Target->getTriple().isAArch64()) {
-      // TargetClones for AArch64
       llvm::SmallVector<StringRef, 8> Feats;
       TC->getFeatures(Feats, GD.getMultiVersionIndex());
-      for (StringRef Feat : Feats)
-        if (Target->validateCpuSupports(Feat.str()))
-          // Use '?' to mark features that came from AArch64 TargetClones.
-          Features.push_back("?" + Feat.str());
+      std::vector<std::string> Features = getFMVBackendFeaturesFor(Feats);
       Features.insert(Features.begin(),
                       Target->getTargetOpts().FeaturesAsWritten.begin(),
                       Target->getTargetOpts().FeaturesAsWritten.end());
+      Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
     } else {
+      std::vector<std::string> Features;
       StringRef VersionStr = TC->getFeatureStr(GD.getMultiVersionIndex());
       if (VersionStr.starts_with("arch="))
         TargetCPU = VersionStr.drop_front(sizeof("arch=") - 1);
       else if (VersionStr != "default")
         Features.push_back((StringRef{"+"} + VersionStr).str());
+      Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
     }
-    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
   } else if (const auto *TV = FD->getAttr<TargetVersionAttr>()) {
-    std::vector<std::string> Feats = filterFunctionTargetVersionAttrs(TV);
-    Feats.insert(Feats.begin(),
-                 Target->getTargetOpts().FeaturesAsWritten.begin(),
-                 Target->getTargetOpts().FeaturesAsWritten.end());
-    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Feats);
+    llvm::SmallVector<StringRef, 8> Feats;
+    TV->getFeatures(Feats);
+    std::vector<std::string> Features = getFMVBackendFeaturesFor(Feats);
+    Features.insert(Features.begin(),
+                    Target->getTargetOpts().FeaturesAsWritten.begin(),
+                    Target->getTargetOpts().FeaturesAsWritten.end());
+    Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
   } else {
     FeatureMap = Target->getTargetOpts().FeatureMap;
   }
diff --git a/clang/lib/AST/CMakeLists.txt b/clang/lib/AST/CMakeLists.txt
index a5d3dacfc1a84..0328666d59b1f 100644
--- a/clang/lib/AST/CMakeLists.txt
+++ b/clang/lib/AST/CMakeLists.txt
@@ -139,4 +139,6 @@ add_clang_library(clangAST
   omp_gen
   ClangDriverOptions
   intrinsics_gen
+  # These generated headers are included transitively.
+  AArch64TargetParserTableGen
   )
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 08d13c41a4857..6fba5fff7bcc1 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1052,57 +1052,18 @@ bool AArch64TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
   return true;
 }
 
-bool AArch64TargetInfo::initFeatureMap(
-    llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
-    const std::vector<std::string> &FeaturesVec) const {
-  std::vector<std::string> UpdatedFeaturesVec;
-  // Parse the CPU and add any implied features.
-  std::optional<llvm::AArch64::CpuInfo> CpuInfo = llvm::AArch64::parseCpu(CPU);
-  if (CpuInfo) {
-    auto Exts = CpuInfo->getImpliedExtensions();
-    std::vector<StringRef> CPUFeats;
-    llvm::AArch64::getExtensionFeatures(Exts, CPUFeats);
-    for (auto F : CPUFeats) {
-      assert((F[0] == '+' || F[0] == '-') && "Expected +/- in target feature!");
-      UpdatedFeaturesVec.push_back(F.str());
-    }
-  }
-
-  // Process target and dependent features. This is done in two loops collecting
-  // them into UpdatedFeaturesVec: first to add dependent '+'features, second to
-  // add target '+/-'features that can later disable some of features added on
-  // the first loop. Function Multi Versioning features begin with '?'.
-  for (const auto &Feature : FeaturesVec)
-    if (((Feature[0] == '?' || Feature[0] == '+')) &&
-        AArch64TargetInfo::doesFeatureAffectCodeGen(Feature.substr(1))) {
-      StringRef DepFeatures =
-          AArch64TargetInfo::getFeatureDependencies(Feature.substr(1));
-      SmallVector<StringRef, 1> AttrFeatures;
-      DepFeatures.split(AttrFeatures, ",");
-      for (auto F : AttrFeatures)
-        UpdatedFeaturesVec.push_back(F.str());
-    }
-  for (const auto &Feature : FeaturesVec)
-    if (Feature[0] != '?') {
-      std::string UpdatedFeature = Feature;
-      if (Feature[0] == '+') {
-        std::optional<llvm::AArch64::ExtensionInfo> Extension =
-            llvm::AArch64::parseArchExtension(Feature.substr(1));
-        if (Extension)
-          UpdatedFeature = Extension->Feature.str();
-      }
-      UpdatedFeaturesVec.push_back(UpdatedFeature);
-    }
-
-  return TargetInfo::initFeatureMap(Features, Diags, CPU, UpdatedFeaturesVec);
-}
-
 // Parse AArch64 Target attributes, which are a comma separated list of:
 //  "arch=<arch>" - parsed to features as per -march=..
 //  "cpu=<cpu>" - parsed to features as per -mcpu=.., with CPU set to <cpu>
 //  "tune=<cpu>" - TuneCPU set to <cpu>
 //  "feature", "no-feature" - Add (or remove) feature.
 //  "+feature", "+nofeature" - Add (or remove) feature.
+//
+// A feature may correspond to an Extension (anything with a corresponding
+// AEK_), in which case an ExtensionSet is used to parse it and expand its
+// dependencies. Otherwise the feature is passed through (e.g. +v8.1a,
+// +outline-atomics, -fmv, etc). Features coming from the command line are
+// already parsed, therefore their dependencies do not need expansion.
 ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
   ParsedTargetAttr Ret;
   if (Features == "default")
@@ -1112,23 +1073,26 @@ ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
   bool FoundArch = false;
 
   auto SplitAndAddFeatures = [](StringRef FeatString,
-                                std::vector<std::string> &Features) {
+                                std::vector<std::string> &Features,
+                                llvm::AArch64::ExtensionSet &FeatureBits) {
     SmallVector<StringRef, 8> SplitFeatures;
     FeatString.split(SplitFeatures, StringRef("+"), -1, false);
     for (StringRef Feature : SplitFeatures) {
-      StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
-      if (!FeatureName.empty())
-        Features.push_back(FeatureName.str());
+      if (FeatureBits.parseModifier(Feature, /* AllowNoDashForm = */ true))
+        continue;
+      // Pass through features that are not extensions, e.g. +v8.1a,
+      // +outline-atomics, -fmv, etc.
+      if (Feature.starts_with("no"))
+        Features.push_back("-" + Feature.drop_front(2).str());
       else
-        // Pushing the original feature string to give a sema error later on
-        // when they get checked.
-        if (Feature.starts_with("no"))
-          Features.push_back("-" + Feature.drop_front(2).str());
-        else
-          Features.push_back("+" + Feature.str());
+        Features.push_back("+" + Feature.str());
     }
   };
 
+  llvm::AArch64::ExtensionSet FeatureBits;
+  // Reconstruct the bitset from the command line option features.
+  FeatureBits.reconstructFromParsedFeatures(getTargetOpts().FeaturesAsWritten);
+
   for (auto &Feature : AttrFeatures) {
     Feature = Feature.trim();
     if (Feature.starts_with("fpmath="))
@@ -1151,9 +1115,9 @@ ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
       // Ret.Features.
       if (!AI)
         continue;
-      Ret.Features.push_back(AI->ArchFeature.str());
+      FeatureBits.addArchDefaults(*AI);
       // Add any extra features, after the +
-      SplitAndAddFeatures(Split.second, Ret.Features);
+      SplitAndAddFeatures(Split.second, Ret.Features, FeatureBits);
     } else if (Feature.starts_with("cpu=")) {
       if (!Ret.CPU.empty())
         Ret.Duplicate = "cpu=";
@@ -1163,7 +1127,10 @@ ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
         std::pair<StringRef, StringRef> Split =
             Feature.split("=").second.trim().split("+");
         Ret.CPU = Split.first;
-        SplitAndAddFeatures(Split.second, Ret.Features);
+        if (auto CpuInfo = llvm::AArch64::parseCpu(Ret.CPU)) {
+          FeatureBits.addCPUDefaults(*CpuInfo);
+          SplitAndAddFeatures(Split.second, Ret.Features, FeatureBits);
+        }
       }
     } else if (Feature.starts_with("tune=")) {
       if (!Ret.Tune.empty())
@@ -1171,25 +1138,19 @@ ParsedTargetAttr AArch64TargetInfo::parseTargetAttr(StringRef Features) const {
       else
         Ret.Tune = Feature.split("=").second.trim();
     } else if (Feature.starts_with("+")) {
-      SplitAndAddFeatures(Feature, Ret.Features);
-    } else if (Feature.starts_with("no-")) {
-      StringRef FeatureName =
-          llvm::AArch64::getArchExtFeature(Feature.split("-").second);
-      if (!FeatureName.empty())
-        Ret.Features.push_back("-" + FeatureName.drop_front(1).str());
-      else
-        Ret.Features.push_back("-" + Feature.split("-").second.str());
+      SplitAndAddFeatures(Feature, Ret.Features, FeatureBits);
     } else {
-      // Try parsing the string to the internal target feature name. If it is
-      // invalid, add the original string (which could already be an internal
-      // name). These should be checked later by isValidFeatureName.
-      StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
-      if (!FeatureName.empty())
-        Ret.Features.push_back(FeatureName.str());
+      if (FeatureBits.parseModifier(Feature, /* AllowNoDashForm = */ true))
+        continue;
+      // Pass through features that are not extensions, e.g. +v8.1a,
+      // +outline-atomics, -fmv, etc.
+      if (Feature.starts_with("no-"))
+        Ret.Features.push_back("-" + Feature.drop_front(3).str());
       else
         Ret.Features.push_back("+" + Feature.str());
     }
   }
+  FeatureBits.toLLVMFeatureList(Ret.Features);
   return Ret;
 }
 
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 12fb50286f751..696553ef8038a 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -107,10 +107,6 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
   unsigned multiVersionSortPriority(StringRef Name) const override;
   unsigned multiVersionFeatureCost() const override;
 
-  bool
-  initFeatureMap(llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags,
-                 StringRef CPU,
-                 const std::vector<std::string> &FeaturesVec) const override;
   bool useFP16ConversionIntrinsics() const override {
     return false;
   }
diff --git a/clang/test/CodeGen/aarch64-cpu-supports-target.c b/clang/test/CodeGen/aarch64-cpu-supports-target.c
index e023944b24e53..28187bcf74533 100644
--- a/clang/test/CodeGen/aarch64-cpu-supports-target.c
+++ b/clang/test/CodeGen/aarch64-cpu-supports-target.c
@@ -48,5 +48,5 @@ int test_versions() {
     return code();
 }
 // CHECK: attributes #0 = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// CHECK: attributes #1 = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+neon" }
-// CHECK: attributes #2 = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon,+sve" }
+// CHECK: attributes #1 = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+neon" }
+// CHECK: attributes #2 = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+sve" }
diff --git a/clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp b/clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
index af8933d93d6cb..9885ac45e6a0e 100644
--- a/clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
+++ b/clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme \
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -target-feature +bf16 \
 // RUN:   -disable-O0-optnone -Werror -emit-llvm -o - %s \
 // RUN: | opt -S -passes=mem2reg \
 // RUN: | opt -S -passes=inline \
diff --git a/clang/test/CodeGen/aarch64-targetattr.c b/clang/test/CodeGen/aarch64-targetattr.c
index 3e7a209245607..644e6a692c3be 100644
--- a/clang/test/CodeGen/aarch64-targetattr.c
+++ b/clang/test/CodeGen/aarch64-targetattr.c
@@ -58,58 +58,50 @@ void v1msve() {}
 // CHECK-LABEL: @plussve() #12
 __attribute__((target("+sve")))
 void plussve() {}
-// CHECK-LABEL: @plussveplussve2() #13
+// CHECK-LABEL: @plussveplussve2() #12
 __attribute__((target("+sve+nosve2")))
 void plussveplussve2() {}
-// CHECK-LABEL: @plussveminusnosve2() #13
+// CHECK-LABEL: @plussveminusnosve2() #12
 __attribute__((target("sve,no-sve2")))
 void plussveminusnosve2() {}
-// CHECK-LABEL: @plusfp16() #14
+// CHECK-LABEL: @plusfp16() #13
 __attribute__((target("+fp16")))
 void plusfp16() {}
 
-// CHECK-LABEL: @all() #15
+// CHECK-LABEL: @all() #14
 __attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2")))
 void all() {}
-// CHECK-LABEL: @allplusbranchprotection() #16
+// CHECK-LABEL: @allplusbranchprotection() #15
 __attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2,branch-protection=standard")))
 void allplusbranchprotection() {}
 
-// These tests check that the user facing and internal llvm name are both accepted.
-// CHECK-LABEL: @plusnoneon() #17
-__attribute__((target("+noneon")))
-void plusnoneon() {}
-// CHECK-LABEL: @plusnosimd() #17
+// CHECK-LABEL: @plusnosimd() #16
 __attribute__((target("+nosimd")))
 void plusnosimd() {}
-// CHECK-LABEL: @noneon() #17
-__attribute__((target("no-neon")))
-void noneon() {}
-// CHECK-LABEL: @nosimd() #17
+// CHECK-LABEL: @nosimd() #16
 __attribute__((target("no-simd")))
 void nosimd() {}
 
 // This isn't part of the standard interface, but test that -arch features should not apply anything else.
-// CHECK-LABEL: @minusarch() #18
+// CHECK-LABEL: @minusarch() #17
 __attribute__((target("no-v9.3a")))
 void minusarch() {}
 
 // CHECK: attributes #0 = { {{.*}} "target-features"="+crc,+fp-armv8,+lse,+neon,+ras,+rdm,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #1 = { {{.*}} "target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #2 = { {{.*}} "target-features"="+crc,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rdm,+sve,+sve2,+v8.1a,+v8.2a,+v8a" }
-// CHECK: attributes #3 = { {{.*}} "target-features"="+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+ras,+rcpc,+rdm,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" }
-// CHECK: attributes #4 = { {{.*}} "target-cpu"="cortex-a710" "target-features"="+bf16,+complxnum,+crc,+dotprod,+flagm,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+jsconv,+lse,+mte,+neon,+pauth,+ras,+rcpc,+rdm,+sb,+sve,+sve2,+sve2-bitperm" }
+// CHECK: attributes #3 = { {{.*}} "target-features"="+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+ras,+rcpc,+rdm,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" }
+// CHECK: attributes #4 = { {{.*}} "target-cpu"="cortex-a710" "target-features"="+bf16,+complxnum,+crc,+dotprod,+flagm,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+jsconv,+lse,+mte,+neon,+pauth,+ras,+rcpc,+rdm,+sb,+sve,+sve2,+sve2-bitperm,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8a,+v9a" }
 // CHECK: attributes #5 = { {{.*}} "tune-cpu"="cortex-a710" }
 // CHECK: attributes #6 = { {{.*}} "target-cpu"="generic" }
 // CHECK: attributes #7 = { {{.*}} "tune-cpu"="generic" }
-// CHECK: attributes #8 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+aes,+crc,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs" "tune-cpu"="cortex-a710" }
-// CHECK: attributes #9 = { {{.*}} "target-features"="+fp-armv8,+fullfp16,+neon,+sve" "tune-cpu"="cortex-a710" }
-// CHECK: attributes #10 = { {{.*}} "target-cpu"="neoverse-v1" "target-features"="+aes,+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+rand,+ras,+rcpc,+rdm,+sha2,+sha3,+sm4,+spe,+ssbs,+sve,+sve2" }
-// CHECK: attributes #11 = { {{.*}} "target-cpu"="neoverse-v1" "target-features"="+aes,+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+rand,+ras,+rcpc,+rdm,+sha2,+sha3,+sm4,+spe,+ssbs,-sve" }
-// CHECK: attributes #12 = { {{.*}} "target-features"="+fp-armv8,+fullfp16,+neon,+sve" }
-// CHECK: attributes #13 = { {{.*}} "target-features"="+fp-armv8,+fullfp16,+neon,+sve,-sve2" }
-// CHECK: attributes #14 = { {{.*}} "target-features"="+fullfp16" }
-// CHECK: attributes #15 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+aes,+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
-// CHECK: attributes #16 = { {{.*}} "branch-target-enforcement"="true" "guarded-control-stack"="true" {{.*}} "target-features"="+aes,+bf16,+complxnum,+crc,+dotprod,+fp-armv8,+fullfp16,+i8mm,+jsconv,+lse,+neon,+pauth,+ras,+rcpc,+rdm,+sha2,+spe,+ssbs,+sve,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
-// CHECK: attributes #17 = { {{.*}} "target-features"="-neon" }
-// CHECK: attributes #18 = { {{.*}} "target-features"="-v9.3a" }
+// CHECK: attributes #8 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+aes,+crc,...
[truncated]

clang/lib/AST/ASTContext.cpp Outdated Show resolved Hide resolved
* Revert inserting the Driver features in front of the features
  of a parsed target attribute.
* Instead make reconstructFromParsedFeatures() add unrecognized
  features to the features of a parsed target attribute.
* Do not allow 'no-' form when parsing '+' separated features.
* Updated misleading comments and added FIXMEs.
@labrinea labrinea requested a review from tmatheson-arm June 14, 2024 11:38
@labrinea labrinea changed the title Parse target attribute Reland "[AArch64] Decouple feature dependency expansion. (#94279)" Jun 14, 2024
Copy link
Contributor

@tmatheson-arm tmatheson-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tmatheson-arm and others added 2 commits June 18, 2024 11:42
Fixes three tests which fail after the rebase by adding -target-feature +sve.
@labrinea labrinea merged commit a03d06a into llvm:main Jun 18, 2024
7 checks passed
@labrinea labrinea deleted the parse-target-attribute branch June 18, 2024 20:28
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
llvm#95519)

This is the second attempt. When parsing the target attribute
we should be letting cc1 features which don't correspond to
Extensions pass through to avoid errors like the following:

% cat neon.c
__attribute__((target("arch=armv8-a")))
uint64x2_t foo(uint64x2_t a, uint64x2_t b) { return veorq_u64(a, b); }

% clang --target=aarch64-linux-gnu -c neon.c
error: always_inline function 'veorq_u64' requires target feature
       'outline-atomics', but would be inlined into function 'foo'
       that is compiled without support for 'outline-atomics'

Co-authored-by: Tomas Matheson <Tomas.Matheson@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants